Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix page size #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix page size #123

wants to merge 1 commit into from

Conversation

James23rc
Copy link

Size of a page is 4096 bytes, which is 8 * 512 bytes, not 4096 * 512 bytes(which is 2MB).

@James23rc
Copy link
Author

@huaicheng

@inhoinno
Copy link
Contributor

inhoinno commented Oct 14, 2023

Hello James23rc,
Thanks for your valid point out. I acknowledged your commit.

However, I have little worries about hardcoding the numbers such as /4096, /8. (I know /4096 things are we did, but still)
So I'm considering not to take your commit, at the moment.
Please see the reason below.

Size of the page in SSD is also a configurable parameter(Scalability)
and hardcoding such as ( lba / 8 ) hinders understanding the code.(Code visibility)

Using macro something like this is better, this is just an example

#define _4KB 4096 
#define LBA_TO_LPN (SSD_PAGE_SIZE / HOST_LBA_SIZE)

Again, appreciate for your commit, we will reflect your point soon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants